Skip to content

dynamic-lambda: continue tracking the best proposal even after freezing#5701

Merged
cce merged 275 commits intomasterfrom
yossi/credentialProp
Oct 13, 2023
Merged

dynamic-lambda: continue tracking the best proposal even after freezing#5701
cce merged 275 commits intomasterfrom
yossi/credentialProp

Conversation

@yossigi
Copy link
Copy Markdown
Contributor

@yossigi yossigi commented Aug 22, 2023

Summary

This PR builds on #5654 and changes the credential propagation and tracking strategy, so we continue propagating credentials for some time after the round concluded and continue to track when we received the lowest credential. This makes the measurements for the dynamic lambda feature independent of the actual filter timeout being set.

Test Plan

Extending existing tests to make sure that better period 0 step 0 credentials get propagated even if they're late.
Extending existing tests to make sure that better period 0 step 0 credentials impact the measurements used in setting the filter timeout.

yossigi and others added 30 commits August 10, 2023 16:43
remove commented out code from when we were trying to get receivedAt for proposal-votes
fix comment to say validated instead of received
Comment thread agreement/player.go Outdated
cce and others added 2 commits October 11, 2023 09:55
Co-authored-by: Jason Paulos <jasonpaulos@users.noreply.github.com>
Comment thread agreement/proposalManager.go Outdated
gmalouf
gmalouf previously approved these changes Oct 11, 2023
Copy link
Copy Markdown
Contributor

@gmalouf gmalouf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated naming conventions made this noticeably easier to read/follow. There is a codegen failure to address on the build.

Comment thread agreement/events.go Outdated
Comment thread agreement/events.go Outdated
Comment thread agreement/service.go Outdated
Copy link
Copy Markdown
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM -- it appears the build error is because msgp generation needs to be run again

jannotti
jannotti previously approved these changes Oct 11, 2023
@cce cce dismissed stale reviews from jannotti and gmalouf via 5923d76 October 12, 2023 00:21
@cce cce merged commit 4249027 into master Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants